test: census of standalone toHaveBeenCalledWith sites where the call count is the contract (#15607) - #15782
Conversation
A census of `toHaveBeenCalledWith` across `packages/**`, `examples/**` and `apps/**` found 811 assertions, 763 of them standalone — no sibling `toHaveBeenCalledTimes` / `toHaveBeenCalledOnce` / `mock.calls.length` on the same spy in the same test. Most of those genuinely do not care about arity and are left alone on purpose. Nine do care, and all nine sit on a path where a doubled call is a defect the assertion cannot see: irreversible byte deletes in a reap sweep, a retry backoff, and seed application — the class the card was filed from. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zGPuVVX3deAx9LdjK8jCk
…lled-with-count-census
📓 Docs Drift CheckNothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 0 changed package(s)), so this run has no opinion about the docs. What this run could not see
Coarse fallback — 0 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): |
|
ACCEPT — PR #15782 (head The ruling's shape held: the population is large (811 Measured by this seat: 9 added count assertions read from the diff; only Positive control accepted on PR #15605's head blob rather than main (that PR is still open) — stated, not hidden. Outcome per the ruling: the class is small; no follow-up cards owed. Flip + arm when both required jobs report success on Generated by Claude Code |
|
Flipped ready + auto-merge enabled (08:03Z, Generated by Claude Code |
Fixes #15607
The card is an existence proof, not a population: one measured instance where a
standalone
toHaveBeenCalledWithhid a doubledql.insert. So the first job was tosize the class, and only then to act on the part of it where the count is actually
the contract.
1. The census
Rule. A
toHaveBeenCalledWithassertion is standalone when notoHaveBeenCalledTimes/toHaveBeenCalledOnce/SPY.mock.calls.length/SPY.mock.calls+toHaveLengthsits on the same spy in the sameit(/test(block. Scanned: every
*.test.*/*.spec.*file underpackages/**,examples/**,apps/**, with comments and literal content masked through the repo's ownscripts/js-comment-mask.mjs, so a mention inside a docblock or a string is not countedas an assertion.
Result on
origin/main:toHaveBeenCalledWithassertions total.not.toHaveBeenCalledWith(asserts absence — a repeat cannot satisfy it)Per package (standalone, on
origin/main):⇒ The population is large. That is the card's own predicted branch, and it is why
this PR is a census plus nine pins, not a sweep. 742 assertions that do not care
about arity are left exactly as they are: adding a count where none is meant would pin a
number nobody chose.
Controls
Positive control — the #15262 seed rig must come back COVERED where it now counts
calls. PR #15605 is still open, so this had to be run against its head blob, not
origin/main(onorigin/mainthat file has no count at all):11 minus 8 = the 3 assertions #15605 pinned alongside its
toHaveBeenCalledTimes(its lines 327, 345, 346) — scored covered, and no others. Control holds.
Negative control — the site the card actually measured,
packages/runtime/src/app-plugin.seed.test.ts:78(insert), is found by the scan andappears in the standalone list. Control holds.
Two blind spots the scan had, both found by reconciling its own total against a raw
count and both fixed before any number above was reported (a census that cannot see
part of its corpus is the very failure this card is about):
expect.soft(spy)was not recognised as anexpectcall — 1 assertion invisible. Thatsite turned out to be a one-shot leader-fence test, i.e. a strong candidate.
expect(value, message)two-argument form folded the message into the spy key,so a spy counted with a message and asserted without one read as two different
spies. That reported a genuinely covered site (
db-job-adapter.once-leader.test.ts,which asserts
toHaveBeenCalledTimes(2)two lines above) as standalone.After both fixes the parsed total (811) equals the masked-code total exactly, so no
occurrence in the corpus is unaccounted for.
Method (reproducible)
Not added under
scripts/— it earns no gate, so per the card it lives here. Save ascensus.mjsat the repo root and runnode census.mjs [--sites|--json].2. Classification — would a doubled call be a defect here?
Applied to the standalone population. The honest answer for the overwhelming majority is
no: they assert the shape of one call (a route forwarding its argument, a log line, a
one-shot lookup) and arity is not part of what they pin.
YES — nine sites, all one class: a repeat is a silent correctness bug and nothing else
in the test can see it. These are pinned in this PR.
packages/services/service-storage/src/attachment-lifecycle.test.ts:424s.delete.../attachment-lifecycle.test.ts:476s.delete.../attachment-lifecycle.test.ts:577s.deletepackages/services/service-storage/src/lax-deviation-reclamation-gate.test.ts:188s.delete.../lax-deviation-reclamation-gate.test.ts:209s.deletepackages/spec/src/shared/resilient-fetch.test.ts:69sleepfetchImpl.packages/runtime/src/seed-loader.test.ts:1123engine.insertmode: 'insert'⇒ one write..../seed-loader.test.ts:1158engine.insert.../seed-loader.test.ts:1190engine.insertEach of the three files already establishes this idiom itself —
attachment-lifecycle.test.ts:548and
:564,resilient-fetch.test.tsthroughout,seed-loader.test.ts:309— so thecorrect form was fixed by existing evidence, not invented here.
Deliberately NOT pinned, though they sit in card-named categories: the ~30
.not.toHaveBeenCalledWithsites (they assert absence already); route-forwarding andregistration assertions (
registerService,registerFlow,dispatch) where the testpins the argument shape and no once-semantics exists in the code under test; and
packages/runtime/src/app-plugin.seed.test.tsitself, which is both out of scopeper the card and held by the open PR #15605.
3. Ablation — the count assertions were driven red, and the thesis measured
Mutation: double every byte delete in the reap guard
(
packages/services/service-storage/src/attachment-lifecycle.ts, both call sites), i.e.exactly the defect shape the card describes. Trap-guarded, absolute paths.
The test files import
./attachment-lifecycle.js, which resolves to the package's ownsrc/under vitest — nodistis involved, so no rebuild is needed for the mutation totake effect, and none was performed.
Mutation proven on disk before measuring (not by an editor's exit code): injected marker
x2, un-doubled call sites remainingx0,git diff --statshowing2 insertions(+), 2 deletions(-). A first attempt matched zero anchors; the script refused to run thesuite and exited 93 rather than report a reading from an unmutated tree.
With this PR's pins:
All six failures are
AssertionError: expected "vi.fn()" to be called 1 times, but got 2 times.Baseline leg — the same mutation with the two test files rolled back to
origin/main(this PR's pins removed):
⇒ Under a doubled irreversible byte delete, the tree today reds one test.
lax-deviation-reclamation-gate.test.tsgoes entirely green. Every standalonetoHaveBeenCalledWithin both files passes under the doubling — which is the card'sthesis, measured rather than restated.
Restore proven both legs:
git checkout HEAD -- ABSOLUTE_PATH, thengit hash-objectequal to the HEAD blob (2e0349f5…) andgit diff HEADempty.4. Verification
Run at the final commit
0e9dad969.pnpm --filter @objectstack/service-storage exec vitest run src/attachment-lifecycle.test.ts src/lax-deviation-reclamation-gate.test.ts→Tests 55 passed (55)(exit 0)pnpm --filter @objectstack/spec exec vitest run src/shared/resilient-fetch.test.ts→Tests 9 passed (9)(exit 0)pnpm --filter @objectstack/runtime exec vitest run src/seed-loader.test.ts→Tests 41 passed (41)(exit 0)typecheck+check:test-typecheckfor all three packages → exit 0. The test layer iscovered explicitly:
check:test-typecheck: OK — @objectstack/service-storage's test layer compiles under packages/services/service-storage/tsconfig.test.json; 0 file(s) / 0 error(s),and likewise for
@objectstack/specand@objectstack/runtime.node scripts/pm/dispatch-gates.mjs --changed --commands --repo objectstack-ai/objectstack→EXIT=0,69 commands, derived from the merged tree (the first derivation warned STALE TREE
against 3 changed family files;
origin/mainwas merged in and it was derived again —same 69).
pnpm check:nul-bytes→check-nul-bytes: OK (scanned 7615 text file(s) ... no raw ASCII control bytes).Five
@objectstack/specgates first returned "build first"; afterpnpm --filter @objectstack/spec buildall five are exit 0.node scripts/pm/check-governed-merges.mjs --teston the final four paths →✅ NOT governed — ordinary queue landing applies to a PR with exactly this file list.NOT MEASURED (exit 3 — prerequisite not met, a whole-repo
pnpm build; these aredeclared to CI, not read as passes):
pnpm check:dual-build-cjs-loads,pnpm check:i18n,pnpm check:type-check-debt.Repo-wide
pnpm lintwas not run locally — it is CI's, per the local-scope rule.Changeset: none, and
skip-changesetapplied. AGENTS.md: "that label is for a diffthat publishes nothing from any released package." This diff is four
*.test.tsfiles,and none of the three packages ships tests — their
filesare["dist", "README.md", "CHANGELOG.md"](@objectstack/specadditionallysrc/**/*.zod.ts, which a.test.tsis not).🤖 Generated with Claude Code
https://claude.ai/code/session_012zGPuVVX3deAx9LdjK8jCk
Generated by Claude Code